configuration: make AbsolutePath a wrapper type#15362
configuration: make AbsolutePath a wrapper type#15362amaanq wants to merge 1 commit intoNixOS:masterfrom
AbsolutePath a wrapper type#15362Conversation
| * rejecting empty and relative paths. | ||
| */ | ||
| struct AbsolutePath : std::filesystem::path | ||
| struct AbsolutePath |
There was a problem hiding this comment.
Can be done using private inheritance, no? Less churny code to write, just using path::operator== and stuff.
There was a problem hiding this comment.
Hm I tried this, and it looks like we'd still have churn because we can't just do using path::operator== and friends because they are defined as non-member friend functions
amaanq
force-pushed
the
convert-path-setting-followup
branch
from
906aabb to
029f54c
Compare
amaanq
force-pushed
the
convert-path-setting-followup
branch
from
029f54c to
7de2b59
Compare
Public inheritance from `std::filesystem::path` lets `AbsolutePath` silently slice down to a plain path when passed by value, so this commit changes `AbsolutePath` to use a `path` field instead, which is easier to reason about and prevents that.
amaanq
force-pushed
the
convert-path-setting-followup
branch
from
7de2b59 to
125d6f3
Compare
|
|
||
| if (auto & caFile = fileTransfer.settings.caFile.get()) | ||
| curl_easy_setopt(req, CURLOPT_CAINFO, caFile->c_str()); | ||
| curl_easy_setopt(req, CURLOPT_CAINFO, caFile->string().c_str()); |
There was a problem hiding this comment.
Can you double-check the docs for curl wrt to lifetime requirements on these temporaries? I think it copies some arguments, so this might be fine, but worth double-checking.
| /* If no file exist in the specified path, curl continues to work | ||
| anyway as if netrc support was disabled. */ | ||
| curl_easy_setopt(req, CURLOPT_NETRC_FILE, fileTransfer.settings.netrcFile.get().c_str()); | ||
| curl_easy_setopt(req, CURLOPT_NETRC_FILE, fileTransfer.settings.netrcFile.get().string().c_str()); |
| { | ||
| using path::path; | ||
| using path::operator=; | ||
| std::filesystem::path path; |
There was a problem hiding this comment.
make this private? With just an accessor? This will help ensure the invariant.
| AbsolutePath(std::filesystem::path p) | ||
| : path(std::move(p)) | ||
| { | ||
| } | ||
|
|
||
| AbsolutePath(std::filesystem::path && p) | ||
| : path(std::move(p)) | ||
| AbsolutePath(const char * s) | ||
| : path(s) | ||
| { | ||
| } |
There was a problem hiding this comment.
The constructors should then barf if the path is not absolute, right?
2 participants
Motivation
Public inheritance from
std::filesystem::pathletsAbsolutePathsilently slice down to a plain path when passed by value, so this commit changes
AbsolutePathto use apathfield instead, which is easier to reason about and prevents that.Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.